battery-embed: add recipe #22698

toge · 2024-02-07T01:01:53Z

Specify library name and version: battery-embed/*

Is cmake module only package acceptable?

Let me first check if it is OK to create a recipe with only a cmake module.

Since ignition-cmake also provides a cmake module, I would like to achieve the same thing with battery-embed.
https://github.com/conan-io/conan-center-index/tree/master/recipes/ignition-cmake

Is this recipe adequate in cci policy?

patch description

battery-embed currently has the following two problems:

example program is always built
compile error with gcc13 due to lack of cstdint

Patch files try to solve these.

reason two versions are provided

1.2.x requires std::jthread and std::stop_token.
Those are not provided on apple-clang (yet).

I've read the contributing guidelines.
I've used a recent Conan client version close to the currently deployed.
I've tried at least one configuration locally with the conan-center hook activated.

uilianries · 2024-02-09T07:32:29Z

compile error with gcc13 due to lack of cstdint

@toge Do you have a log or an issue/PR in the upstream to confirm about this behavior? I don't have GCC13 installed here right now to validate it 😞

example program is always built

It could be suggested to the upstream to use your patch instead, so users are free to decide about it.

toge · 2024-02-09T14:27:01Z

@uilianries

It could be suggested to the upstream to use your patch instead, so users are free to decide about it.

The issue about it has been created on upstream.
batterycenter/embed#2

I'm watching it.

Do you have a log or an issue/PR in the upstream to confirm about this behavior? I don't have GCC13 installed here right now to validate it 😞

Off course!
Here are:

======== Exporting recipe to the cache ======== WARN: Name containing special chars is discouraged 'battery-embed' battery-embed/1.2.8: Exporting package recipe: /home/toge/src/conan-center-index/recipes/battery-embed/all/conanfile.py battery-embed/1.2.8: exports: File 'conandata.yml' found. Exporting it... battery-embed/1.2.8: Calling export_sources() battery-embed/1.2.8: Copied 1 '.yml' file: conandata.yml battery-embed/1.2.8: Copied 1 '.py' file: conanfile.py battery-embed/1.2.8: Copied 2 '.patch' files: 0001-disable-examples.patch, 0002-include-cstdint.patch battery-embed/1.2.8: Exported to cache folder: /home/toge/.conan2/p/battebcc91ef1dfa80/e battery-embed/1.2.8: Exported: battery-embed/1.2.8#dc406be40a1fbe9305318a815f8cd372 (2024-02-09 09:48:05 UTC)

======== Input profiles ========
Profile host:
[settings]
arch=x86_64
build_type=Release
compiler=gcc
compiler.cppstd=20
compiler.libcxx=libstdc++11
compiler.version=13
os=Linux

Profile build:
[settings]
arch=x86_64
build_type=Release
compiler=gcc
compiler.cppstd=20
compiler.libcxx=libstdc++11
compiler.version=13
os=Linux

======== Computing dependency graph ========
Graph root
cli
Build requirements
battery-embed/1.2.8#dc406be40a1fbe9305318a815f8cd372 - Cache
cmake/3.27.4#a7e78418b024dccacccc887f049f47ed - Cache
Resolved version ranges
cmake/[>=3.21 <4]: cmake/3.27.4

======== Computing necessary packages ========
battery-embed/1.2.8: Compatible package ID 9a4eb3c8701508aa9458b1a73d0633783ecc2270 equal to the default package ID: Skipping it.
Build requirements
battery-embed/1.2.8#dc406be40a1fbe9305318a815f8cd372:9a4eb3c8701508aa9458b1a73d0633783ecc2270 - Build
cmake/3.27.4#a7e78418b024dccacccc887f049f47ed:63fead0844576fc02943e16909f08fcdddd6f44b#fac7109ddc535a744445b6e1c095129c - Cache

======== Installing packages ========
cmake/3.27.4: Already installed! (1 of 2)
cmake/3.27.4: Appending PATH environment variable: /home/toge/.conan2/p/cmake9d32db761075c/p/bin
battery-embed/1.2.8: Calling source() in /home/toge/.conan2/p/battebcc91ef1dfa80/s

-------- Installing package battery-embed/1.2.8 (2 of 2) --------
battery-embed/1.2.8: Building from source
battery-embed/1.2.8: Package battery-embed/1.2.8:9a4eb3c8701508aa9458b1a73d0633783ecc2270
battery-embed/1.2.8: Copying sources to build folder
battery-embed/1.2.8: Building your package in /home/toge/.conan2/p/b/battefe07bce641e42/b
battery-embed/1.2.8: Generating aggregated env files
battery-embed/1.2.8: Generated aggregated env files: ['conanbuild.sh', 'conanrun.sh']
battery-embed/1.2.8: Calling build()
battery-embed/1.2.8: Apply patch (conan): disable building examples
battery-embed/1.2.8: Apply patch (portability): include cstdint for gcc 13 later
battery-embed/1.2.8: Package '9a4eb3c8701508aa9458b1a73d0633783ecc2270' built
battery-embed/1.2.8: Build folder /home/toge/.conan2/p/b/battefe07bce641e42/b
battery-embed/1.2.8: Generating the package
battery-embed/1.2.8: Packaging in folder /home/toge/.conan2/p/b/battefe07bce641e42/p
battery-embed/1.2.8: Calling package()
battery-embed/1.2.8: package(): Packaged 1 file: LICENSE
battery-embed/1.2.8: package(): Packaged 1 '.txt' file: CMakeLists.txt
battery-embed/1.2.8: Created package revision 383d669dd11332c6b6339d753c0fb9eb
battery-embed/1.2.8: Package '9a4eb3c8701508aa9458b1a73d0633783ecc2270' created
battery-embed/1.2.8: Full package reference: battery-embed/1.2.8#dc406be40a1fbe9305318a815f8cd372:9a4eb3c8701508aa9458b1a73d0633783ecc2270#383d669dd11332c6b6339d753c0fb9eb
battery-embed/1.2.8: Package folder /home/toge/.conan2/p/b/battefe07bce641e42/p
WARN: deprecated: Usage of deprecated Conan 1.X features that will be removed in Conan 2.X:
WARN: deprecated: 'env_info' used in: cmake/3.27.4

======== Launching test_package ========

======== Computing dependency graph ========
Graph root
battery-embed/1.2.8 (test package): /home/toge/src/conan-center-index/recipes/battery-embed/all/test_package/conanfile.py
Requirements
battery-embed/1.2.8#dc406be40a1fbe9305318a815f8cd372 - Cache
Build requirements
cmake/3.27.4#a7e78418b024dccacccc887f049f47ed - Cache

======== Computing necessary packages ========
Requirements
battery-embed/1.2.8#dc406be40a1fbe9305318a815f8cd372:9a4eb3c8701508aa9458b1a73d0633783ecc2270#383d669dd11332c6b6339d753c0fb9eb - Cache
Build requirements
Skipped binaries
cmake/3.27.4

======== Installing packages ========
battery-embed/1.2.8: Already installed! (1 of 1)

======== Testing the package ========
Removing previously existing 'test_package' build folder: /home/toge/src/conan-center-index/recipes/battery-embed/all/test_package/build/gcc-13-x86_64-20-release
battery-embed/1.2.8 (test package): Test package build: build/gcc-13-x86_64-20-release
battery-embed/1.2.8 (test package): Test package build folder: /home/toge/src/conan-center-index/recipes/battery-embed/all/test_package/build/gcc-13-x86_64-20-release
battery-embed/1.2.8 (test package): Writing generators to /home/toge/src/conan-center-index/recipes/battery-embed/all/test_package/build/gcc-13-x86_64-20-release/generators
battery-embed/1.2.8 (test package): Generator 'VirtualRunEnv' calling 'generate()'
battery-embed/1.2.8 (test package): Generator 'CMakeToolchain' calling 'generate()'
battery-embed/1.2.8 (test package): CMakeToolchain generated: conan_toolchain.cmake
battery-embed/1.2.8 (test package): CMakeToolchain generated: CMakePresets.json
battery-embed/1.2.8 (test package): CMakeToolchain generated: ../../../CMakeUserPresets.json
battery-embed/1.2.8 (test package): Generator 'CMakeDeps' calling 'generate()'
battery-embed/1.2.8 (test package): CMakeDeps necessary find_package() and targets for your CMakeLists.txt
find_package(battery-embed)
target_link_libraries(... battery-embed::battery-embed)
battery-embed/1.2.8 (test package): Generating aggregated env files
battery-embed/1.2.8 (test package): Generated aggregated env files: ['conanrun.sh', 'conanbuild.sh']

======== Testing the package: Building ========
battery-embed/1.2.8 (test package): Calling build()
battery-embed/1.2.8 (test package): Running CMake.configure()
battery-embed/1.2.8 (test package): RUN: cmake -G "Unix Makefiles" -DCMAKE_TOOLCHAIN_FILE="/home/toge/src/conan-center-index/recipes/battery-embed/all/test_package/build/gcc-13-x86_64-20-release/generators/conan_toolchain.cmake" -DCMAKE_INSTALL_PREFIX="/home/toge/src/conan-center-index/recipes/battery-embed/all/test_package" -DCMAKE_POLICY_DEFAULT_CMP0091="NEW" -DCMAKE_BUILD_TYPE="Release" "/home/toge/src/conan-center-index/recipes/battery-embed/all/test_package"
-- Using Conan toolchain: /home/toge/src/conan-center-index/recipes/battery-embed/all/test_package/build/gcc-13-x86_64-20-release/generators/conan_toolchain.cmake
-- Conan toolchain: C++ Standard 20 with extensions OFF
-- The CXX compiler identification is GNU 13.2.1
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Conan: Target declared 'battery-embed::battery-embed'
-- Conan: Including build module from '/home/toge/.conan2/p/b/battefe07bce641e42/p/lib/cmake/battery-embed/CMakeLists.txt'
-- Configuring done (0.2s)
-- Generating done (0.0s)
-- Build files have been written to: /home/toge/src/conan-center-index/recipes/battery-embed/all/test_package/build/gcc-13-x86_64-20-release

battery-embed/1.2.8 (test package): Running CMake.build()
battery-embed/1.2.8 (test package): RUN: cmake --build "/home/toge/src/conan-center-index/recipes/battery-embed/all/test_package/build/gcc-13-x86_64-20-release" -- -j12
[ 20%] Generating embed/autogen/test_package_cpp.cpp
[ 40%] Building CXX object CMakeFiles/test_package.dir/test_package.cpp.o
[ 60%] Building CXX object CMakeFiles/test_package.dir/embed/autogen/test_package_cpp.cpp.o
[ 80%] Building CXX object CMakeFiles/test_package.dir/embed/embed_impl.cpp.o
[100%] Linking CXX executable test_package
[100%] Built target test_package

======== Testing the package: Executing test ========
battery-embed/1.2.8 (test package): Running test()
battery-embed/1.2.8 (test package): RUN: ./test_package
#include
#include
#include

#include "battery/embed.hpp"

int main(void) {
std::cout << b::embed<"test_package.cpp">() << std::endl;
return EXIT_SUCCESS;
}

valgur

Looks like a perfectly reasonable solution to me. Thanks!

HerrNamenlos123 · 2024-04-05T09:38:27Z

Hi! I am the author of battery-embed.

Interesting that people tried to get my library into Conan, without me even knowing!
I apprechiate the effort, however. I would have been more than willing to make upstream changes if they are for the greater good.

Are there still any issues remaining? I added <cstdint> upstream, no issue with that. Also, examples are only built if battery-embed is the top level CMake module AND BUILD_EXAMPLES is set (which it is, if it's the top-level module).

Is there still an issue with std::jthread? It's only used for convenience. If necessary, it can easily be changed to a normal std::thread and std::atomic<bool> instead of std::stop_token...

AbrilRBS · 2024-04-05T10:45:12Z

Hi @HerrNamenlos123 I'll get back to you with the status of the PR later today, just wanted to drop by to thank you for offering to help, really appreciated :)

HerrNamenlos123 · 2024-04-05T20:26:01Z

@toge I really apprechiate your efforts. Could you explain to me why you want to add 1.2.12 AND 1.1.1? I don't see any reason to use 1.1.1? I see that 1.0.0 gives you C++14, but does 1.1.1 give you any advantage over the latest version? If so, can we re-implement it in the latest version?

Also, I don't quite understand why you need to patch the CMakeLists to disable the examples. Can't you define -DBUILD_EXAMPLES=OFF in the conanfile? Isn't that the whole point?

(In case you didn't know, the option() you patched is just defaulted with PROJECT_IS_TOP_LEVEL, meaning the default is based on how you build it. The point of the option is that you can still define it manually and override the default)

I think there shouldn't be any reason to patch anything and if so, i can fix it upstream to keep everything clean...

EDIT: I meant 1.1.1

toge · 2024-04-06T07:34:55Z

@HerrNamenlos123
Thank you for your comment!
I am glad to discuss the way to package your product.

Could you explain to me why you want to add 1.2.12 AND 1.1.1? I don't see any reason to use 1.1.1?

The reason to provide 1.1.1 is supporting apple-clang.
As you know, 1.2.x uses std::jthread.
It seems that std::jthread is not supported by apple-clang, and I get the following error message.

embed/embed_impl.cpp:51:10: error: no type named 'jthread' in namespace 'std'; did you mean 'thread'?
    std::jthread embeddedFileWatcherThread = std::jthread([](std::stop_token stopToken) {
    ~~~~~^~~~~~~
         thread
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.4.sdk/usr/include/c++/v1/__thread/thread.h:164:33: note: 'thread' declared here
class _LIBCPP_EXPORTED_FROM_ABI thread
                                ^
embed/embed_impl.cpp:51:67: error: no type named 'stop_token' in namespace 'std'
    std::jthread embeddedFileWatcherThread = std::jthread([](std::stop_token stopToken) {
                                                             ~~~~~^
embed/embed_impl.cpp:51:51: error: no member named 'jthread' in namespace 'std'
    std::jthread embeddedFileWatcherThread = std::jthread([](std::stop_token stopToken) {
                                             ~~~~~^
embed/embed_impl.cpp:55:40: error: variable '__begin2' cannot be implicitly captured in a lambda with no capture-default specified
            for (auto& [key, filedata] : embeddedFiles) {
                                       ^

Also, I don't quite understand why you need to patch the CMakeLists to disable the examples. Can't you define -DBUILD_EXAMPLES=OFF in the conanfile? Isn't that the whole point?

The reason for patching instead of defining BUILD_EXAMPLES is portability.
I think BUILD_EXAMPLES is a popular name and I am afraid of conflicts.

I think there shouldn't be any reason to patch anything and if so, i can fix it upstream to keep everything clean...

I agree with you.
To solve these problems, I am planning to split CMakeLists.txt into two files.

CMakeLists.txt : project, BUILD_EXAMPLES, add_subdirectory.
battery_embed.cmake : functions and dependent variables.

Does this fit your policy?

recipes/battery-embed/all/conanfile.py

Co-authored-by: Martin Valgur <martin.valgur@gmail.com>

toge · 2024-08-06T10:21:51Z

@valgur
Thanks!

HerrNamenlos123 · 2024-08-08T22:28:58Z

@toge I took a look at the current package and I propose 2 more changes:

Update to the latest Version of 1.2.19, as some important bug fixes were made.
In the Test package, do not include cstdint and cstdlib, because both should be included in embed.hpp itself. Their absence is a bug and the test package should point out the bug and not hide it.

If any other things pop up in the latest version, feel free to immediately create an issue or a PR.

…nan-center-index into battery-embed-add-recipe

toge · 2024-08-12T05:30:01Z

@HerrNamenlos123
Thank you for your comment!
I fixed.

AbrilRBS

Thanks! I have some questions that are confusiong me a bit, otherwise looks good

recipes/battery-embed/all/conanfile.py

AbrilRBS

Thanks! I have some questions that are confusiong me a bit, otherwise looks good

conan-center-bot · 2024-08-21T09:39:18Z

Conan v1 pipeline ✔️

All green in build 17 (fe43ce39c386c9165ad9de09d62a9193f97b2dd5):

battery-embed/1.2.19:
All packages built successfully! (All logs)

Conan v2 pipeline ✔️

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

All green in build 17 (fe43ce39c386c9165ad9de09d62a9193f97b2dd5):

battery-embed/1.2.19:
All packages built successfully! (All logs)

AbrilRBS

Thanks!

HerrNamenlos123 · 2024-08-21T15:56:31Z

Thank you everyone for the effort, was not expecting my library to end up in Conan Center xD

HerrNamenlos123 · 2024-08-21T21:50:26Z

I am terribly sorry for only noticing now, but unfortunately I found another mistake. The Test package links to battery::embed, but this is not supposed to happen. Like with the include paths, this is meant to happen automatically via b_embed().

The package itself works fine, so it's absolutely not critical, but if we want to be pedantic and ensure correct testing, it should be fixed at some point. Sorry for not noticing earlier, I must have skipped over it.

battery-embed: add recipe

f21ebf7

conan-center-bot added the Failed label Feb 7, 2024

This comment has been minimized.

Sign in to view

change package_type

3d81fb3

This comment has been minimized.

Sign in to view

AbrilRBS self-assigned this Feb 7, 2024

toge mentioned this pull request Feb 9, 2024

add battery-embed to KB-H014 allowlist conan-io/hooks#527

Merged

remove cmake_find_mode

76dfcec

This comment has been minimized.

Sign in to view

Merge branch 'master' into battery-embed-add-recipe

d17d0ee

This comment has been minimized.

Sign in to view

link pthread, require newer cmake on test_package

0c3ccc3

This comment has been minimized.

Sign in to view

drop support apple-clang

fdee0a0

conan-center-bot removed the Failed label Feb 16, 2024

This comment has been minimized.

Sign in to view

prince-chrismc mentioned this pull request Feb 16, 2024

Pull Requests Ready to be Reviewed prince-chrismc/conan-center-index-pending-review#1

Open

add 1.1.1 for macOS

a4c66b0

This comment has been minimized.

Sign in to view

conan-center-bot requested review from uilianries and franramirez688 February 23, 2024 11:42

valgur approved these changes Mar 24, 2024

View reviewed changes

update 1.2.12

dea66e3

This comment has been minimized.

Sign in to view

conan-center-bot requested a review from AbrilRBS July 30, 2024 11:46

valgur reviewed Aug 5, 2024

View reviewed changes

recipes/battery-embed/all/conanfile.py Outdated Show resolved Hide resolved

valgur reviewed Aug 5, 2024

View reviewed changes

recipes/battery-embed/all/conanfile.py Outdated Show resolved Hide resolved

valgur reviewed Aug 5, 2024

View reviewed changes

recipes/battery-embed/all/conanfile.py Outdated Show resolved Hide resolved

valgur approved these changes Aug 5, 2024

View reviewed changes

toge and others added 3 commits August 6, 2024 19:20

remove export_coinandata_patches

572579a

Co-authored-by: Martin Valgur <martin.valgur@gmail.com>

remove apply_conandata_patches

a3179b6

Co-authored-by: Martin Valgur <martin.valgur@gmail.com>

remove unused import

6d744de

Co-authored-by: Martin Valgur <martin.valgur@gmail.com>

This comment has been minimized.

Sign in to view

toge added 2 commits August 12, 2024 14:27

update 1.2.19

bb07f1e

Merge branch 'battery-embed-add-recipe' of https://github.com/toge/co…

8eddb62

…nan-center-index into battery-embed-add-recipe

This comment has been minimized.

Sign in to view

danimtb previously approved these changes Aug 20, 2024

View reviewed changes

danimtb self-assigned this Aug 20, 2024

AbrilRBS reviewed Aug 20, 2024

View reviewed changes

recipes/battery-embed/all/conanfile.py Show resolved Hide resolved

recipes/battery-embed/all/conanfile.py Show resolved Hide resolved

AbrilRBS reviewed Aug 20, 2024

View reviewed changes

Update recipes/battery-embed/all/conanfile.py

fe43ce3

danimtb dismissed their stale review via fe43ce3 August 21, 2024 09:21

valgur approved these changes Aug 21, 2024

View reviewed changes

danimtb requested a review from AbrilRBS August 21, 2024 10:46

danimtb approved these changes Aug 21, 2024

View reviewed changes

AbrilRBS approved these changes Aug 21, 2024

View reviewed changes

conan-center-bot merged commit f4ac88d into conan-io:master Aug 21, 2024
12 checks passed

HerrNamenlos123 mentioned this pull request Aug 21, 2024

battery-embed: Remove link to battery::embed in test package #25009

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

battery-embed: add recipe #22698

battery-embed: add recipe #22698

toge commented Feb 7, 2024 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

uilianries commented Feb 9, 2024

toge commented Feb 9, 2024 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

valgur left a comment

HerrNamenlos123 commented Apr 5, 2024

AbrilRBS commented Apr 5, 2024

This comment has been minimized.

HerrNamenlos123 commented Apr 5, 2024 •

edited

Loading

toge commented Apr 6, 2024

toge commented Aug 6, 2024

This comment has been minimized.

HerrNamenlos123 commented Aug 8, 2024

toge commented Aug 12, 2024

This comment has been minimized.

AbrilRBS left a comment

AbrilRBS left a comment

conan-center-bot commented Aug 21, 2024

AbrilRBS left a comment

HerrNamenlos123 commented Aug 21, 2024

HerrNamenlos123 commented Aug 21, 2024

battery-embed: add recipe #22698

battery-embed: add recipe #22698

Conversation

toge commented Feb 7, 2024 • edited Loading

Is cmake module only package acceptable?

patch description

reason two versions are provided

This comment has been minimized.

This comment has been minimized.

uilianries commented Feb 9, 2024

toge commented Feb 9, 2024 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

valgur left a comment

Choose a reason for hiding this comment

HerrNamenlos123 commented Apr 5, 2024

AbrilRBS commented Apr 5, 2024

This comment has been minimized.

HerrNamenlos123 commented Apr 5, 2024 • edited Loading

toge commented Apr 6, 2024

toge commented Aug 6, 2024

This comment has been minimized.

HerrNamenlos123 commented Aug 8, 2024

toge commented Aug 12, 2024

This comment has been minimized.

AbrilRBS left a comment

Choose a reason for hiding this comment

AbrilRBS left a comment

Choose a reason for hiding this comment

conan-center-bot commented Aug 21, 2024

Conan v1 pipeline ✔️

Conan v2 pipeline ✔️

AbrilRBS left a comment

Choose a reason for hiding this comment

HerrNamenlos123 commented Aug 21, 2024

HerrNamenlos123 commented Aug 21, 2024

toge commented Feb 7, 2024 •

edited

Loading

toge commented Feb 9, 2024 •

edited

Loading

HerrNamenlos123 commented Apr 5, 2024 •

edited

Loading